Sync Sprint 1 exercises with the error-handling prep - #1460
Conversation
Add mean and describe-median katas to implement, and change the existing fix/implement exercises to throw on bad input instead of ignoring it, matching the updated Sprint 1 prep.
abdishakoor-dev
added
the
NotCoursework
| [1, 2, "3", null, undefined, 4], | ||
| ["apple", 1, 2, 3, "banana", 4], | ||
| [1, "2", 3, "4", 5], | ||
| [1, "apple", 2, null, 3, undefined, 4], | ||
| [3, "apple", 1, null, 2, undefined, 4], | ||
| ["banana", 5, 3, "apple", 1, 4, 2], | ||
| ].forEach((input) => | ||
| it(`throws for an array containing a non-number [${input}]`, () => | ||
| expect(() => calculateMedian(input)).toThrow( | ||
| new Error("calculateMedian requires an array of numbers") | ||
| )) |
There was a problem hiding this comment.
To throwback to a short conversation we had months ago, I'm not a fan of this setup for running tests for multiple reasons, namely:
- Difficult syntax for trainees who won't be familiar with
forEachto parse - Produces an output in the Jest CLI that is confusing
I recognise this is a problem for the happy path tests in this sprint as well. What do you think? Do you want to make changes here or should we put an issue in to refactor all tests to not use this structure?
| // Given an empty array | ||
| // When passed to describeMedian | ||
| // Then it should not throw |
There was a problem hiding this comment.
Given that there is already a test described in comments above covering the behaviour of the function when given an empty array, what new behaviour is this test attempting to cover? The above tests check its return value, which I feel implicitly verifies it doesn't throw (if it did, there would be no return). What do you think?
|
|
||
| In this kata, you will need to implement a function that calculates the mean of an array of numbers. | ||
|
|
||
| E.g. calculateMean([1, 2, 3]), target output: 2 |
There was a problem hiding this comment.
Nitpick: for this example input the mean is 2 but the median is also 2. Technically there isn't anything incorrect about this, but I could imagine people who are less mathematically confident to get mixed up by seeing the 2 in the middle of the array and thinking they understand what the "mean" is.
Perhaps calculateMean([1, 2, 6]), target output: 3 would avoid this risk?
| // When passed to calculateMean | ||
| // Then it should return their mean | ||
| // Delete this test.todo and replace it with a test. | ||
| test.todo("given [1, 2, 3], returns 2"); |
|
@abdishakoor-dev Looks good! I think this will be a really helpful addition to people and it follows on nicely from the prep (which is very clearly written). Just a few small comments for your consideration 🙂 |
|
median.test.js: describe-median.test.js: mean.test.js line 5: mean.test.js line 26: |
2 participants
The Sprint 1 prep now covers throwing errors and try/catch, but the coursework still asked trainees to ignore non-numbers and return null. This brings the exercises in line with it.
The median, max and sum katas now expect a named error on bad input instead of a made-up value. Two new implement katas are added: mean, which throws when it can't calculate, and describe-median, which catches the error and returns a message.
Relevant Prep: https://curriculum.codeyourfuture.io/itp/data-groups/sprints/1/prep/#throwing-errors